feat: smarter exception messaging — deduplicated chains, format_for_display, log_record#278
Merged
bashandbone merged 3 commits intomainfrom Mar 25, 2026
Merged
Conversation
…isplay, log_record Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> Agent-Logs-Url: https://github.com/knitli/codeweaver/sessions/972c309c-a994-4516-95d9-c482a5cf2ad1
Contributor
Reviewer's GuideRefactors CodeWeaver exception rendering so str is concise, adds explicit display/logging helpers, and updates the CLI error handler to walk exception chains, surface root causes, and deduplicate suggestions/boilerplate. Sequence diagram for CLI handling of a CodeWeaverError chainsequenceDiagram
actor User
participant CLICommand
participant CLIErrorHandler
participant ChainHelpers as _collect_codeweaver_chain
participant ExternalRootHelper as _get_external_root
participant SuggestionHelper as _deduplicate_suggestions
participant DisplayConsole as Console
User->>CLICommand: invoke_command()
CLICommand->>CLICommand: raise CodeWeaverError from cause_chain
CLICommand->>CLIErrorHandler: handle_error(error, context, exit_code)
CLIErrorHandler->>CLIErrorHandler: isinstance(error, CodeWeaverError)
CLIErrorHandler->>CLIErrorHandler: _handle_codeweaver_error(error, context)
CLIErrorHandler->>ChainHelpers: _collect_codeweaver_chain(error)
ChainHelpers-->>CLIErrorHandler: list_of_CodeWeaverError_nodes
CLIErrorHandler->>DisplayConsole: print(prefix and context_failed_header)
CLIErrorHandler->>CLIErrorHandler: _print_primary_error(error)
CLIErrorHandler->>DisplayConsole: print(primary_error_message_and_location)
CLIErrorHandler->>DisplayConsole: print(details_if_present)
alt chain_has_deeper_causes
CLIErrorHandler->>CLIErrorHandler: _print_cause_chain(causes)
CLIErrorHandler->>DisplayConsole: print(condensed_cause_lines)
end
CLIErrorHandler->>ExternalRootHelper: _get_external_root(error)
ExternalRootHelper-->>CLIErrorHandler: external_root_or_none
alt external_root_present
CLIErrorHandler->>DisplayConsole: print(underlying_cause_line)
end
CLIErrorHandler->>SuggestionHelper: _deduplicate_suggestions(all_suggestions_from_chain)
SuggestionHelper-->>CLIErrorHandler: unique_suggestions
alt suggestions_present
CLIErrorHandler->>DisplayConsole: print(Suggestions_header)
CLIErrorHandler->>DisplayConsole: print(bulleted_suggestions)
end
CLIErrorHandler->>DisplayConsole: print(issue_reporting_boilerplate_once)
alt verbose_or_debug
CLIErrorHandler->>DisplayConsole: print(full_traceback)
end
DisplayConsole-->>User: sees_compact_chain_with_deduplicated_boilerplate
Class diagram for updated CodeWeaverError and CLI error handlingclassDiagram
class LocationInfo {
+str filename
+int line_number
+str module_name
+from_frame(depth: int) LocationInfo
}
class CodeWeaverError {
+str message
+dict details
+list suggestions
+LocationInfo location
+__str__() str
+format_for_display(include_suggestions: bool, include_details: bool, include_issue_info: bool) str
+log_record() dict
}
class InitializationError {
}
class CLIErrorHandler {
+bool verbose
+bool debug
+display
+str prefix
+handle_error(error: Exception, context: str, exit_code: int) None
+_handle_codeweaver_error(error: CodeWeaverError, context: str) None
+_print_primary_error(error: CodeWeaverError) None
+_print_cause_chain(causes: list) None
}
class _collect_codeweaver_chain {
<<function>>
+_collect_codeweaver_chain(exc: BaseException) list
}
class _get_external_root {
<<function>>
+_get_external_root(exc: BaseException) BaseException
}
class _deduplicate_suggestions {
<<function>>
+_deduplicate_suggestions(suggestions: list) list
}
CodeWeaverError *-- LocationInfo : has_location
InitializationError --|> CodeWeaverError : inherits
CLIErrorHandler ..> CodeWeaverError : handles
CLIErrorHandler ..> _collect_codeweaver_chain : uses
CLIErrorHandler ..> _get_external_root : uses
CLIErrorHandler ..> _deduplicate_suggestions : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Copilot created this pull request from a session on behalf of
bashandbone
March 25, 2026 13:55
View session
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
_BULLETconstant is now duplicated in bothexceptions.pyanderror_handler.py; consider defining it in a single module and importing it to avoid divergence in future changes. - The new helper functions are exported via
__all__but retain leading underscores (e.g._collect_codeweaver_chain); consider either dropping the underscores or omitting them from__all__to make the intended public/private surface clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_BULLET` constant is now duplicated in both `exceptions.py` and `error_handler.py`; consider defining it in a single module and importing it to avoid divergence in future changes.
- The new helper functions are exported via `__all__` but retain leading underscores (e.g. `_collect_codeweaver_chain`); consider either dropping the underscores or omitting them from `__all__` to make the intended public/private surface clearer.
## Individual Comments
### Comment 1
<location path="src/codeweaver/cli/ui/error_handler.py" line_range="31-40" />
<code_context>
+_BULLET = "\u2022"
+
+
+def _collect_codeweaver_chain(exc: BaseException) -> list[CodeWeaverError]:
+ """Walk ``__cause__`` / ``__context__`` and return all CodeWeaverError nodes.
+
+ Returns nodes in outermost-first order (the exception itself is first).
+ Stops following the chain when it hits a non-CodeWeaverError or exceeds
+ ``_MAX_CHAIN_DEPTH`` to guard against unexpectedly long or cyclic chains.
+
+ Args:
+ exc: The exception to start from.
+
+ Returns:
+ List of CodeWeaverError instances from outermost to root.
+ """
+ chain: list[CodeWeaverError] = []
+ seen: set[int] = set()
+ current: BaseException | None = exc
+ while current is not None and len(chain) < _MAX_CHAIN_DEPTH:
+ exc_id = id(current)
+ if exc_id in seen:
</code_context>
<issue_to_address>
**issue (bug_risk):** Depth guard and docstring for `_collect_codeweaver_chain` don't match actual behavior and may miss very long non-CodeWeaver chains.
The implementation currently continues traversing through non-CodeWeaverError nodes (only filtering them out of `chain`), and the depth guard is keyed to `len(chain)` (CodeWeaverError count) rather than total steps. As a result, a long chain of non-CodeWeaverError causes/contexts can still be fully traversed and `_MAX_CHAIN_DEPTH` doesn’t actually bound traversal.
Either (a) add a separate step counter / `for _ in range(_MAX_CHAIN_DEPTH)` loop to cap total nodes visited, or (b) adjust the docstring to describe that only CodeWeaverError depth is limited and non-CodeWeaver chains may be arbitrarily long.
</issue_to_address>
### Comment 2
<location path="src/codeweaver/cli/ui/error_handler.py" line_range="63-72" />
<code_context>
+def _get_external_root(exc: BaseException) -> BaseException | None:
</code_context>
<issue_to_address>
**issue:** `_get_external_root` returns the first non-CodeWeaverError in the chain, not necessarily the root cause as described.
The current logic returns the first non-`CodeWeaverError` after `exc`, not the deepest one. For a chain like `CodeWeaverError -> ValueError -> OSError`, this would return `ValueError`, even though the docstring/log message describe the “root/lowest-level” cause.
Either:
- Change the implementation to walk the entire chain, track the last non-`CodeWeaverError`, and return that; or
- Keep the current behavior but update the docstring and log wording to reflect that this is the first external exception, not the root cause.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+63
to
+72
| def _get_external_root(exc: BaseException) -> BaseException | None: | ||
| """Return the first non-CodeWeaverError exception at the root of the chain. | ||
|
|
||
| This surfaces the original third-party or built-in exception (e.g. | ||
| ``OSError``, ``ImportError``) that triggered the CodeWeaver chain so users | ||
| can see what actually went wrong at the lowest level. | ||
|
|
||
| Args: | ||
| exc: The outermost exception. | ||
|
|
Contributor
There was a problem hiding this comment.
issue: _get_external_root returns the first non-CodeWeaverError in the chain, not necessarily the root cause as described.
The current logic returns the first non-CodeWeaverError after exc, not the deepest one. For a chain like CodeWeaverError -> ValueError -> OSError, this would return ValueError, even though the docstring/log message describe the “root/lowest-level” cause.
Either:
- Change the implementation to walk the entire chain, track the last non-
CodeWeaverError, and return that; or - Keep the current behavior but update the docstring and log wording to reflect that this is the first external exception, not the root cause.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors how CodeWeaverError instances are rendered so exception chains don’t repeatedly print the same boilerplate, while adding explicit APIs for user-facing rendering and structured logging. It updates the CLI error handler to display a concise cause chain and aggregate suggestions/issue-reporting information once.
Changes:
- Simplifies
CodeWeaverError.__str__and introducesformat_for_display()andlog_record(). - Reworks CLI error rendering to collect/deduplicate
CodeWeaverErrorchains, show condensed causes, and print boilerplate once. - Adds helper utilities for chain walking, external root detection, and suggestion deduplication.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/codeweaver/core/exceptions.py | Adds display/logging APIs and changes default string rendering to avoid repeated boilerplate in exception chains. |
| src/codeweaver/cli/ui/error_handler.py | Implements chain-aware CLI rendering with condensed causes, aggregated suggestions, and single boilerplate output. |
Comments suppressed due to low confidence (1)
src/codeweaver/core/exceptions.py:195
format_for_display(include_details=True)only renders a hard-coded subset of keys fromself.details, and renders nothing ifdetailscontains other keys. This doesn’t match the docstring (“Include the details dict”) and can silently drop important context. Consider rendering all detail key/values (with the special-case ordering/labels as an enhancement) or falling back to a generic dump when unknown keys are present.
if include_details and self.details:
detail_parts: list[str] = []
if "file_path" in self.details:
detail_parts.append(f"file: {self.details['file_path']}")
detail_parts.extend(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+27
to
+29
| _MAX_CHAIN_DEPTH = 10 | ||
| _BULLET = "\u2022" | ||
|
|
There was a problem hiding this comment.
_BULLET is duplicated here and in codeweaver.core.exceptions (and the PR description implies it’s shared). If you want a single source of truth, import it from the core module or move it to a dedicated constants module; otherwise the two copies can drift and produce inconsistent output.
Comment on lines
+301
to
+303
| "_collect_codeweaver_chain", | ||
| "_deduplicate_suggestions", | ||
| "_get_external_root", |
There was a problem hiding this comment.
__all__ is exporting underscore-prefixed helper functions (_collect_codeweaver_chain, etc.), which effectively makes private helpers part of the module’s public API. Either drop these from __all__ (keep them private) or rename them without the leading underscore and add them to the package’s managed exports if they’re intended to be public.
Suggested change
| "_collect_codeweaver_chain", | |
| "_deduplicate_suggestions", | |
| "_get_external_root", |
Comment on lines
+221
to
+225
| def log_record(self) -> dict[str, Any]: | ||
| """Return a structured record for use with structured logging systems. | ||
|
|
||
| Produces a plain ``dict`` containing all exception data so that | ||
| logging back-ends (structlog, Python logging with a JSON formatter, |
There was a problem hiding this comment.
New behavior/API (log_record()) isn’t exercised anywhere in the repo yet. Add unit tests that validate the returned dict shape/keys (and a couple representative values) to prevent regressions as exception logging evolves.
Comment on lines
+184
to
+188
| chain = _collect_codeweaver_chain(error) | ||
|
|
||
| self.display.console.print(f"\n{self.prefix}\n [bold red]✗ {context} failed[/bold red]\n") | ||
| self._print_primary_error(error) | ||
|
|
There was a problem hiding this comment.
The updated chain-rendering behavior (deduplicated suggestions, condensed causes, external root surfacing) isn’t covered by existing CLIErrorHandler tests. Add unit tests that build a CodeWeaverError chain with raise ... from ... and assert boilerplate/suggestions appear only once and causes are rendered as condensed → lines.
Comment on lines
+52
to
+53
| if isinstance(current, CodeWeaverError): | ||
| chain.append(current) |
There was a problem hiding this comment.
_collect_codeweaver_chain is intended to stop at _MAX_CHAIN_DEPTH, but the loop condition uses len(chain) < _MAX_CHAIN_DEPTH. If the walk reaches a non-CodeWeaverError, chain stops growing and the function can traverse an arbitrarily long external __cause__/__context__ chain, defeating the cap. Track traversal depth separately (or break on the first non-CodeWeaverError) so the cap is enforced.
Suggested change
| if isinstance(current, CodeWeaverError): | |
| chain.append(current) | |
| if not isinstance(current, CodeWeaverError): | |
| # Stop walking once we hit a non-CodeWeaverError to avoid traversing | |
| # arbitrarily long external chains. | |
| break | |
| chain.append(current) |
| self.display.console.print(f" {_BULLET} {suggestion}") | ||
| self.display.console.print() | ||
|
|
||
| for line in CodeWeaverError._issue_information: |
There was a problem hiding this comment.
The CLI handler is reaching into CodeWeaverError._issue_information, which is a private implementation detail. To avoid coupling, expose a public API for this (e.g., a CodeWeaverError.issue_information() accessor or a format_issue_info() helper) and have the CLI call that instead.
Suggested change
| for line in CodeWeaverError._issue_information: | |
| for line in CodeWeaverError.issue_information: |
Contributor
|
@copilot apply changes based on the comments in this thread Also incorporate the following feedback: Please address the comments from this code review: Overall Comments
Individual CommentsComment 1+_BULLET = "\u2022" + + +def _collect_codeweaver_chain(exc: BaseException) -> list[CodeWeaverError]: + """Walk ``__cause__`` / ``__context__`` and return all CodeWeaverError nodes. + + Returns nodes in outermost-first order (the exception itself is first). + Stops following the chain when it hits a non-CodeWeaverError or exceeds + ``_MAX_CHAIN_DEPTH`` to guard against unexpectedly long or cyclic chains. + + Args: + exc: The exception to start from. + + Returns: + List of CodeWeaverError instances from outermost to root. + """ + chain: list[CodeWeaverError] = [] + seen: set[int] = set() + current: BaseException | None = exc + while current is not None and len(chain) < _MAX_CHAIN_DEPTH: + exc_id = id(current) + if exc_id in seen: **issue (bug_risk):** Depth guard and docstring for `_collect_codeweaver_chain` don't match actual behavior and may miss very long non-CodeWeaver chains.The implementation currently continues traversing through non-CodeWeaverError nodes (only filtering them out of Either (a) add a separate step counter / Comment 2+def _get_external_root(exc: BaseException) -> BaseException | None: **issue:** `_get_external_root` returns the first non-CodeWeaverError in the chain, not necessarily the root cause as described.The current logic returns the first non- Either:
|
…ublic issue_information, add tests Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> Agent-Logs-Url: https://github.com/knitli/codeweaver/sessions/9803a9d1-bd50-4705-907c-d9f87891473c
Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> Agent-Logs-Url: https://github.com/knitli/codeweaver/sessions/9803a9d1-bd50-4705-907c-d9f87891473c
Contributor
Author
Addressed all review feedback in commits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the "wall of boilerplate" problem where a
CodeWeaverErrorexception chain previously repeated the full GitHub issue links, alpha notice, location details, and suggestions at every level. A 4-deep chain could show the same advice a dozen times. Now it's shown exactly once.Problem
CodeWeaverError.__str__embedded the entire user-facing display (location, details, GitHub links, alpha notice) so every exception in a Pythonraise X from Ychain would format itself in full when printed or logged, producing walls of repeated boilerplate.The
CLIErrorHandleralso separately re-rendereddetailsandsuggestionson top of callingstr(error)(which already contained them), doubling the output even for a single exception.Changes
src/codeweaver/core/exceptions.py_BULLET— module-level constant (•); imported by the CLI handler so both share a single source of truth.issue_information— publicClassVar(previously_issue_information) exposing the issue-reporting boilerplate lines without coupling callers to a private attribute.CodeWeaverError.__str__simplified to"{message} (in '{module}', line N)"— clean for logs and tracebacks, no boilerplate.CodeWeaverError.format_for_display()— new method that renders a single exception node with full context. Three keyword flags:include_details— include thedetailsdictinclude_suggestions— include thesuggestionslistinclude_issue_info— append the alpha/issue-reporting boilerplate (passTrueonly on the outermost display call)CodeWeaverError.log_record()— new method that returns a structureddict[str, Any]with all exception data (error_type,message,details,suggestions,location) for use with structlog / JSON log formatters.src/codeweaver/cli/ui/error_handler.py_BULLET— imported fromexceptions.py(no longer duplicated)._collect_codeweaver_chain(exc)— walks__cause__/__context__(respecting__suppress_context__) and returns allCodeWeaverErrornodes outermost-first, capped at depth 10. Breaks immediately on the first non-CodeWeaverErrornode to prevent unbounded traversal of external exception chains._get_external_root(exc)— finds the first non-CodeWeaverErrorin the chain to surface the original third-party exception (OSError,ImportError, etc.)._deduplicate_suggestions(suggestions)— deduplicates while preserving original order._-prefixed) and intentionally excluded from__all__.CLIErrorHandler._handle_codeweaver_errorrewritten:→ ExcType: message (location)line.CodeWeaverError.issue_information._print_primary_errorand_print_cause_chainprivate helpers to keep cyclomatic complexity within linter limits.tests/unit/core/test_exceptions.py(new)37 unit tests covering:
CodeWeaverError.__str__concisenesslog_record()dict shape, key values, and copy semanticsissue_informationpublic ClassVar accessibility_collect_codeweaver_chain: chain order, depth cap, stops at non-CodeWeaverError_get_external_root: external exception detection_deduplicate_suggestions: order-preserving deduplicationCLIErrorHandlerchain rendering: message display, condensed→causes, suggestion deduplication, single boilerplate, verbose traceback flagBefore / After
Before (3-deep chain, each level re-printing everything):
After:
Testing
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.